Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

repolist: Implement JSON output #1522

Merged
merged 2 commits into from
Jun 5, 2024
Merged

repolist: Implement JSON output #1522

merged 2 commits into from
Jun 5, 2024

Conversation

jan-kolarik
Copy link
Member

  • Created shared --json option for requesting output in JSON format
  • Implementation for repo list command

For: #867.

@jan-kolarik
context: Prepare shared json option
03b0b49 
For requesting machine-readable output in JSON format.
@jan-kolarik jan-kolarik mentioned this pull request May 31, 2024
Open
7 tasks
@jan-kolarik jan-kolarik force-pushed the jkolarik/repolist-machine-output branch from e464098 to edeb153 Compare May 31, 2024 12:16
@jan-kolarik
Copy link
Member Author

jan-kolarik commented May 31, 2024 •
edited
Loading

Example output:

[root@71dd3fed55a5 dnf5]# dnf5 repo list --json
[
  {
    "id":"copr:copr.fedorainfracloud.org:rpmsoftwaremanagement:dnf-nightly",
    "name":"Copr repo for dnf-nightly owned by rpmsoftwaremanagement",
    "is_enabled":true
  },
  {
    "id":"copr:copr.fedorainfracloud.org:rpmsoftwaremanagement:test-utils",
    "name":"Copr repo for test-utils owned by rpmsoftwaremanagement",
    "is_enabled":true
  },
  {
    "id":"updates",
    "name":"Fedora 39 - x86_64 - Updates",
    "is_enabled":true
  },
  {
    "id":"fedora-cisco-openh264",
    "name":"Fedora 39 openh264 (From Cisco) - x86_64",
    "is_enabled":true
  },
  {
    "id":"fedora",
    "name":"Fedora 39 - x86_64",
    "is_enabled":true
  }
]

@jan-kolarik
Copy link
Member Author

The question is also if to have consistent attributes in the JSON output or if to filter some (like is_enabled) based on the user input.

kontura
kontura reviewed Jun 5, 2024
View reviewed changes
dnf5/context.cpp
@@ -206,6 +209,7 @@ class Context::Impl {
const char * comment{nullptr};

bool should_store_offline = false;
bool json_output = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be stored in the Context?

It seems the value is used only in the commands, to make it simpler I would store it there.
It could use libdnf5::cli::session::BoolOption, for example like advisory SecurityOption?

Copy link
Member Author

@jan-kolarik jan-kolarik Jun 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been already working on the follow-up repoinfo implementation and there I am calling the set_quiet(true) when json output is requested to have really just the JSON on the output, so it's also parsable. In this regard, this place is handy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, however doesn't that have the same issue as mentioned in #1524?
That is dnf5 repo info --quiet --refresh prints REPOSYNC to stdout which would ruin the json.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, you're right this is happening, though I'd think it's suppressed by this line: https://github.com/rpm-software-management/dnf5/blob/5.2.3.0/dnf5/main.cpp#L1192. So it seems like a bug to me right now. I am still planning to do such binding to quiet there...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the issue is that the ^ mentioned callbacks setup should be done after parsing the command line arguments. Then the quiet is taken into account. Tested locally and seems working 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into it.

@kontura
Copy link
Contributor

kontura commented Jun 5, 2024

The question is also if to have consistent attributes in the JSON output or if to filter some (like is_enabled) based on the user input.

Personally I prefer consistent attributes, I believe it will be less error prone.

@kontura kontura added this pull request to the merge queue Jun 5, 2024
Merged via the queue into main with commit 4ffa9d1 Jun 5, 2024
12 of 15 checks passed
@kontura kontura deleted the jkolarik/repolist-machine-output branch June 5, 2024 11:45
@kontura kontura mentioned this pull request Jun 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kontura kontura kontura left review comments

Assignees
No one assigned
Labels
None yet
Projects
DNF team
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jan-kolarik @kontura